Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Service Extension Callout (Envoy external processing) #2895

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

e-n-0
Copy link
Member

@e-n-0 e-n-0 commented Sep 27, 2024

What does this PR do?

Motivation

Reviewer's Checklist

  • Changed code has unit tests for its functionality at or near 100% coverage.
  • System-Tests covering this feature have been added and enabled with the va.b.c-dev version tag.
  • There is a benchmark for any new code, or changes to existing code.
  • If this interacts with the agent in a new way, a system test has been added.
  • Add an appropriate team label so this PR gets put in the right place for the release notes.
  • Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild.

Unsure? Have a question? Request a review!

@DataDog DataDog deleted a comment from datadog-datadog-prod-us1 bot Sep 27, 2024
@DataDog DataDog deleted a comment from datadog-datadog-prod-us1 bot Sep 27, 2024
@DataDog DataDog deleted a comment from datadog-datadog-prod-us1 bot Sep 27, 2024
@DataDog DataDog deleted a comment from datadog-datadog-prod-us1 bot Sep 27, 2024
@DataDog DataDog deleted a comment from datadog-datadog-prod-us1 bot Sep 27, 2024
@DataDog DataDog deleted a comment from datadog-datadog-prod-us1 bot Sep 27, 2024
uses: docker/setup-qemu-action@v3

- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v3

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Code Vulnerability

Workflow depends on a GitHub actions pinned by tag (...read more)

Pin third party actions by hash, or at least by tag for trusted sources

When using a third party action, one needs to provide its GitHub path (owner/project) and can eventually pin it to a git ref (a branch name, a git tag, or a commit hash).

No pinned git ref means the action will use the latest commit of the default branch each time it runs, eventually running newer versions of the code that were not audited by Datadog. Specifying a git tag is better, but since they are not immutable, using a full length hash is recommended to make sure the action content is actually frozen to some reviewed state.

Be careful however, as even pinning an action by hash can be circumvented by attackers still. For instance, if an action relies on a Docker image which is itself not pinned to a digest, it becomes possible to alter its behaviour through the Docker image without actually changing its hash. You can learn more about this kind of attacks in Unpinnable Actions: How Malicious Code Can Sneak into Your GitHub Actions Workflows. Pinning actions by hash is still a good first line of defense against supply chain attacks.

Additionally, pinning by hash or tag means the action won’t benefit from newer version updates if any, including eventual security patches. Make sure to regularly check if newer versions for an action you use are available. For actions coming from a very trustworthy source, it can make sense to use a laxer pinning policy to benefit from updates as soon as possible.

View in Datadog  Leave us feedback  Documentation

id: go

- name: Set up QEMU
uses: docker/setup-qemu-action@v3

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Code Vulnerability

Workflow depends on a GitHub actions pinned by tag (...read more)

Pin third party actions by hash, or at least by tag for trusted sources

When using a third party action, one needs to provide its GitHub path (owner/project) and can eventually pin it to a git ref (a branch name, a git tag, or a commit hash).

No pinned git ref means the action will use the latest commit of the default branch each time it runs, eventually running newer versions of the code that were not audited by Datadog. Specifying a git tag is better, but since they are not immutable, using a full length hash is recommended to make sure the action content is actually frozen to some reviewed state.

Be careful however, as even pinning an action by hash can be circumvented by attackers still. For instance, if an action relies on a Docker image which is itself not pinned to a digest, it becomes possible to alter its behaviour through the Docker image without actually changing its hash. You can learn more about this kind of attacks in Unpinnable Actions: How Malicious Code Can Sneak into Your GitHub Actions Workflows. Pinning actions by hash is still a good first line of defense against supply chain attacks.

Additionally, pinning by hash or tag means the action won’t benefit from newer version updates if any, including eventual security patches. Make sure to regularly check if newer versions for an action you use are available. For actions coming from a very trustworthy source, it can make sense to use a laxer pinning policy to benefit from updates as soon as possible.

View in Datadog  Leave us feedback  Documentation

uses: docker/setup-buildx-action@v3

- name: Login to GitHub Container Registry
uses: docker/login-action@v2

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Code Vulnerability

Workflow depends on a GitHub actions pinned by tag (...read more)

Pin third party actions by hash, or at least by tag for trusted sources

When using a third party action, one needs to provide its GitHub path (owner/project) and can eventually pin it to a git ref (a branch name, a git tag, or a commit hash).

No pinned git ref means the action will use the latest commit of the default branch each time it runs, eventually running newer versions of the code that were not audited by Datadog. Specifying a git tag is better, but since they are not immutable, using a full length hash is recommended to make sure the action content is actually frozen to some reviewed state.

Be careful however, as even pinning an action by hash can be circumvented by attackers still. For instance, if an action relies on a Docker image which is itself not pinned to a digest, it becomes possible to alter its behaviour through the Docker image without actually changing its hash. You can learn more about this kind of attacks in Unpinnable Actions: How Malicious Code Can Sneak into Your GitHub Actions Workflows. Pinning actions by hash is still a good first line of defense against supply chain attacks.

Additionally, pinning by hash or tag means the action won’t benefit from newer version updates if any, including eventual security patches. Make sure to regularly check if newer versions for an action you use are available. For actions coming from a very trustworthy source, it can make sense to use a laxer pinning policy to benefit from updates as soon as possible.

View in Datadog  Leave us feedback  Documentation

@@ -0,0 +1,40 @@
name: Publish Service Extensions Callout images packages

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Code Vulnerability

No explicit permissions set for at the workflow level (...read more)

Check the permissions granted to jobs

Datadog’s GitHub organization defines default permissions for the GITHUB_TOKEN to be restricted (contents:read, metadata:read and packages:read).

Your repository may require different setup, but please consider defining permissions for each job following the least privilege principle to restrict the impact of a possible compromission.

You can find the list of all possible permissions in Workflow syntax for GitHub Actions - GitHub Docs. Please note they can be defined at the job or the workflow level.

View in Datadog  Leave us feedback  Documentation

password: ${{ secrets.GITHUB_TOKEN }}

- name: Build and push
uses: docker/build-push-action@v6

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Code Vulnerability

Workflow depends on a GitHub actions pinned by tag (...read more)

Pin third party actions by hash, or at least by tag for trusted sources

When using a third party action, one needs to provide its GitHub path (owner/project) and can eventually pin it to a git ref (a branch name, a git tag, or a commit hash).

No pinned git ref means the action will use the latest commit of the default branch each time it runs, eventually running newer versions of the code that were not audited by Datadog. Specifying a git tag is better, but since they are not immutable, using a full length hash is recommended to make sure the action content is actually frozen to some reviewed state.

Be careful however, as even pinning an action by hash can be circumvented by attackers still. For instance, if an action relies on a Docker image which is itself not pinned to a digest, it becomes possible to alter its behaviour through the Docker image without actually changing its hash. You can learn more about this kind of attacks in Unpinnable Actions: How Malicious Code Can Sneak into Your GitHub Actions Workflows. Pinning actions by hash is still a good first line of defense against supply chain attacks.

Additionally, pinning by hash or tag means the action won’t benefit from newer version updates if any, including eventual security patches. Make sure to regularly check if newer versions for an action you use are available. For actions coming from a very trustworthy source, it can make sense to use a laxer pinning policy to benefit from updates as soon as possible.

View in Datadog  Leave us feedback  Documentation

runs-on: ubuntu-latest
steps:
- name: Set up Go 1.22
uses: actions/setup-go@v2

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Code Vulnerability

Workflow depends on a GitHub actions pinned by tag (...read more)

Pin third party actions by hash, or at least by tag for trusted sources

When using a third party action, one needs to provide its GitHub path (owner/project) and can eventually pin it to a git ref (a branch name, a git tag, or a commit hash).

No pinned git ref means the action will use the latest commit of the default branch each time it runs, eventually running newer versions of the code that were not audited by Datadog. Specifying a git tag is better, but since they are not immutable, using a full length hash is recommended to make sure the action content is actually frozen to some reviewed state.

Be careful however, as even pinning an action by hash can be circumvented by attackers still. For instance, if an action relies on a Docker image which is itself not pinned to a digest, it becomes possible to alter its behaviour through the Docker image without actually changing its hash. You can learn more about this kind of attacks in Unpinnable Actions: How Malicious Code Can Sneak into Your GitHub Actions Workflows. Pinning actions by hash is still a good first line of defense against supply chain attacks.

Additionally, pinning by hash or tag means the action won’t benefit from newer version updates if any, including eventual security patches. Make sure to regularly check if newer versions for an action you use are available. For actions coming from a very trustworthy source, it can make sense to use a laxer pinning policy to benefit from updates as soon as possible.

View in Datadog  Leave us feedback  Documentation

@e-n-0 e-n-0 force-pushed the flavien/service-extensions branch 4 times, most recently from 40d4289 to 594023c Compare September 27, 2024 13:00
uses: actions/checkout@v4

- name: Set up Go 1.22
uses: actions/setup-go@v5

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Code Vulnerability

Workflow depends on a GitHub actions pinned by tag (...read more)

Pin third party actions by hash, or at least by tag for trusted sources

When using a third party action, one needs to provide its GitHub path (owner/project) and can eventually pin it to a git ref (a branch name, a git tag, or a commit hash).

No pinned git ref means the action will use the latest commit of the default branch each time it runs, eventually running newer versions of the code that were not audited by Datadog. Specifying a git tag is better, but since they are not immutable, using a full length hash is recommended to make sure the action content is actually frozen to some reviewed state.

Be careful however, as even pinning an action by hash can be circumvented by attackers still. For instance, if an action relies on a Docker image which is itself not pinned to a digest, it becomes possible to alter its behaviour through the Docker image without actually changing its hash. You can learn more about this kind of attacks in Unpinnable Actions: How Malicious Code Can Sneak into Your GitHub Actions Workflows. Pinning actions by hash is still a good first line of defense against supply chain attacks.

Additionally, pinning by hash or tag means the action won’t benefit from newer version updates if any, including eventual security patches. Make sure to regularly check if newer versions for an action you use are available. For actions coming from a very trustworthy source, it can make sense to use a laxer pinning policy to benefit from updates as soon as possible.

View in Datadog  Leave us feedback  Documentation

@e-n-0 e-n-0 force-pushed the flavien/service-extensions branch 2 times, most recently from fcbd354 to a587a09 Compare September 27, 2024 13:33
@pr-commenter
Copy link

pr-commenter bot commented Sep 27, 2024

Benchmarks

Benchmark execution time: 2024-10-16 16:38:35

Comparing candidate commit a799cf0 in PR branch flavien/service-extensions with baseline commit 5b9a8af in branch main.

Found 2 performance improvements and 3 performance regressions! Performance is the same for 54 metrics, 0 unstable metrics.

scenario:BenchmarkInjectW3C-24

  • 🟩 execution_time [-204.207ns; -162.793ns] or [-4.753%; -3.789%]

scenario:BenchmarkStartRequestSpan-24

  • 🟥 allocated_mem [+96 bytes; +96 bytes] or [+54.545%; +54.545%]
  • 🟥 allocations [+1; +1] or [+20.000%; +20.000%]
  • 🟥 execution_time [+214.730ns; +215.850ns] or [+71.391%; +71.763%]

scenario:BenchmarkTracerAddSpans-24

  • 🟩 execution_time [-116.970ns; -95.030ns] or [-2.960%; -2.405%]

@e-n-0 e-n-0 force-pushed the flavien/service-extensions branch 8 times, most recently from bdf08fd to 1ef1070 Compare September 30, 2024 12:32
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v4

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Code Vulnerability

Workflow depends on a GitHub actions pinned by tag (...read more)

Pin third party actions by hash, or at least by tag for trusted sources

When using a third party action, one needs to provide its GitHub path (owner/project) and can eventually pin it to a git ref (a branch name, a git tag, or a commit hash).

No pinned git ref means the action will use the latest commit of the default branch each time it runs, eventually running newer versions of the code that were not audited by Datadog. Specifying a git tag is better, but since they are not immutable, using a full length hash is recommended to make sure the action content is actually frozen to some reviewed state.

Be careful however, as even pinning an action by hash can be circumvented by attackers still. For instance, if an action relies on a Docker image which is itself not pinned to a digest, it becomes possible to alter its behaviour through the Docker image without actually changing its hash. You can learn more about this kind of attacks in Unpinnable Actions: How Malicious Code Can Sneak into Your GitHub Actions Workflows. Pinning actions by hash is still a good first line of defense against supply chain attacks.

Additionally, pinning by hash or tag means the action won’t benefit from newer version updates if any, including eventual security patches. Make sure to regularly check if newer versions for an action you use are available. For actions coming from a very trustworthy source, it can make sense to use a laxer pinning policy to benefit from updates as soon as possible.

View in Datadog  Leave us feedback  Documentation

@e-n-0 e-n-0 force-pushed the flavien/service-extensions branch 5 times, most recently from 3940197 to 0a1ae11 Compare September 30, 2024 15:13
@e-n-0 e-n-0 force-pushed the flavien/service-extensions branch 8 times, most recently from 6985de0 to 0c57ea1 Compare September 30, 2024 15:40
Response: &extproc.ProcessingResponse_ImmediateResponse{
ImmediateResponse: &extproc.ImmediateResponse{
Status: &v32.HttpStatus{
Code: v32.StatusCode(currentRequest.statusCode),

Check failure

Code scanning / CodeQL

Incorrect conversion between integer types High

Incorrect conversion of an integer with architecture-dependent bit size from
strconv.Atoi
to a lower bit size type int32 without an upper bound check.

Copilot Autofix AI about 1 hour ago

To fix the problem, we need to ensure that the integer value parsed from the string does not exceed the bounds of int32 before converting it. This can be achieved by using strconv.ParseInt with a bit size of 32, which directly parses the string into an int32 value, or by adding explicit bounds checking after using strconv.Atoi.

The best way to fix this without changing existing functionality is to replace the strconv.Atoi call with strconv.ParseInt specifying a 32-bit size. This ensures that the parsed value is within the bounds of int32.

Suggested changeset 1
contrib/envoyproxy/envoy/envoy.go

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/contrib/envoyproxy/envoy/envoy.go b/contrib/envoyproxy/envoy/envoy.go
--- a/contrib/envoyproxy/envoy/envoy.go
+++ b/contrib/envoyproxy/envoy/envoy.go
@@ -252,3 +252,7 @@
 
-	currentRequest.statusCode, err = strconv.Atoi(statusCodeStr)
+	parsed, err := strconv.ParseInt(statusCodeStr, 10, 32)
+	if err != nil {
+		return nil, status.Errorf(codes.InvalidArgument, "Error parsing response header status code: %v", err)
+	}
+	currentRequest.statusCode = int(parsed)
 	if err != nil {
EOF
@@ -252,3 +252,7 @@

currentRequest.statusCode, err = strconv.Atoi(statusCodeStr)
parsed, err := strconv.ParseInt(statusCodeStr, 10, 32)
if err != nil {
return nil, status.Errorf(codes.InvalidArgument, "Error parsing response header status code: %v", err)
}
currentRequest.statusCode = int(parsed)
if err != nil {
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant